Skip to content
This repository was archived by the owner on Mar 2, 2022. It is now read-only.

Post sealed box & Decrypt response #19

Merged
merged 39 commits into from
Jun 1, 2021
Merged

Conversation

jongbonga
Copy link
Contributor

@jongbonga jongbonga commented May 14, 2021

Issue #13

@jongbonga jongbonga requested review from PiDelport and longtomjr May 14, 2021 17:27
@jongbonga jongbonga self-assigned this May 14, 2021
@jongbonga jongbonga changed the title Post sealed box Post sealed box & Decrypt response May 14, 2021
@jongbonga jongbonga requested review from PiDelport and longtomjr May 20, 2021 21:58
Copy link
Collaborator

@longtomjr longtomjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the vuex actions. It makes sense to think of them as a chain of tasks that needs to be executed. This helps when it comes to naming, and it makes it easier to reason about them. Another thing is that the action that will be called from a component should describe the whole action chain.

For this action chain I can imagine something like this:
Name of the action chain (so the first action name): provisionExecutionToken
and the chain:

provisionExecutionToken(does encryption bit)
-> requestExecutionToken(does the server call bit, and then calls the js function to decrypt response)
-> setExecutionToken (mutation that sets the state)

It does not need to follow this exactly, but just wanted to give an example of what I am talking about.

@jongbonga jongbonga requested a review from longtomjr May 24, 2021 07:46
@PiDelport PiDelport added the enhancement New feature or request label May 26, 2021
@PiDelport
Copy link
Collaborator

PiDelport commented May 26, 2021

I know this is quite a lot of feedback: let me know if you'd like me to tackle some of it, or have a session to go through it?

@PiDelport
Copy link
Collaborator

(Rebased to refresh and update)

Copy link
Collaborator

@PiDelport PiDelport left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jongbonga and I had a session together to resolve the remaining conversations, with @Nghondzweni listening in. Thanks! 😊️

After that, I made to factor out a sealedPost helper substantially simplifies the action code: both encryptAndUploadFile and requestExecutionToken are now just small wrappers around it. @jongbonga and @longtomjr, can you review this change before merging?

@PiDelport PiDelport requested a review from longtomjr June 1, 2021 07:48
Copy link
Collaborator

@longtomjr longtomjr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks everyone for getting this sorted!

@PiDelport PiDelport merged commit 292460e into development Jun 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants